-
Notifications
You must be signed in to change notification settings - Fork 11
fix: Post release channel route #201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughA new OpenAPI specification file has been introduced for the Ctrlplane API, defining the API version as 3.0.0 and outlining a POST endpoint for creating release channels. The request requires a JSON body with specific properties. Additionally, a new POST route has been implemented in the application, incorporating authentication and authorization checks, schema validation, and error handling. The route processes incoming requests by inserting validated data into the database and responding with the created release channel or an error message. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Auth
participant Validator
participant Database
participant API
Client->>API: POST /v1/release-channels
API->>Auth: Authenticate user
Auth-->>API: User authenticated
API->>Validator: Validate request body
Validator-->>API: Validation successful
API->>Database: Insert release channel data
Database-->>API: Return created channel
API-->>Client: Respond with created channel data
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
apps/webservice/src/app/api/v1/release-channels/openapi.ts(1 hunks)apps/webservice/src/app/api/v1/release-channels/route.ts(1 hunks)
🔇 Additional comments (3)
apps/webservice/src/app/api/v1/release-channels/route.ts (2)
1-12: LGTM! Well-organized imports.
The imports are logically grouped and include all necessary dependencies for the implementation.
13-22: LGTM! Well-structured middleware chain with proper security controls.
The middleware chain follows security best practices:
- Authenticates the user
- Validates request data
- Authorizes the action using the validated deploymentId
apps/webservice/src/app/api/v1/release-channels/openapi.ts (1)
3-93: LGTM!
The OpenAPI specification is well-defined and adheres to best practices.
| .handle<{ body: z.infer<typeof createReleaseChannel> }>( | ||
| async ({ db, body }) => | ||
| db | ||
| .insert(SCHEMA.releaseChannel) | ||
| .values(body) | ||
| .returning() | ||
| .then(takeFirst) | ||
| .then((releaseChannel) => NextResponse.json(releaseChannel)) | ||
| .catch((error) => NextResponse.json({ error }, { status: 500 })), | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance error handling and add transaction support.
The current implementation has several areas for improvement:
- Error handling should be more specific to provide better client feedback
- Raw error objects in responses could expose sensitive information
- Missing transaction support for data consistency
Consider applying this improvement:
.handle<{ body: z.infer<typeof createReleaseChannel> }>(
- async ({ db, body }) =>
+ async ({ db, body }) => {
+ try {
+ const releaseChannel = await db.transaction(async (trx) => {
+ const [result] = await trx
+ .insert(SCHEMA.releaseChannel)
+ .values(body)
+ .returning();
+ return result;
+ });
+ return NextResponse.json(releaseChannel);
+ } catch (error) {
+ if (error.code === '23505') { // PostgreSQL unique violation
+ return NextResponse.json(
+ { message: 'Release channel already exists' },
+ { status: 409 }
+ );
+ }
+ console.error('Failed to create release channel:', error);
+ return NextResponse.json(
+ { message: 'Internal server error' },
+ { status: 500 }
+ );
+ }
+ }
- db
- .insert(SCHEMA.releaseChannel)
- .values(body)
- .returning()
- .then(takeFirst)
- .then((releaseChannel) => NextResponse.json(releaseChannel))
- .catch((error) => NextResponse.json({ error }, { status: 500 })),
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .handle<{ body: z.infer<typeof createReleaseChannel> }>( | |
| async ({ db, body }) => | |
| db | |
| .insert(SCHEMA.releaseChannel) | |
| .values(body) | |
| .returning() | |
| .then(takeFirst) | |
| .then((releaseChannel) => NextResponse.json(releaseChannel)) | |
| .catch((error) => NextResponse.json({ error }, { status: 500 })), | |
| ); | |
| .handle<{ body: z.infer<typeof createReleaseChannel> }>( | |
| async ({ db, body }) => { | |
| try { | |
| const releaseChannel = await db.transaction(async (trx) => { | |
| const [result] = await trx | |
| .insert(SCHEMA.releaseChannel) | |
| .values(body) | |
| .returning(); | |
| return result; | |
| }); | |
| return NextResponse.json(releaseChannel); | |
| } catch (error) { | |
| if (error.code === '23505') { // PostgreSQL unique violation | |
| return NextResponse.json( | |
| { message: 'Release channel already exists' }, | |
| { status: 409 } | |
| ); | |
| } | |
| console.error('Failed to create release channel:', error); | |
| return NextResponse.json( | |
| { message: 'Internal server error' }, | |
| { status: 500 } | |
| ); | |
| } | |
| } | |
| ); |
| "500": { | ||
| description: "Failed to create release channel", | ||
| content: { | ||
| "application/json": { | ||
| schema: { | ||
| type: "object", | ||
| properties: { error: { type: "string" } }, | ||
| required: ["error"], | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| "401": { | ||
| description: "Unauthorized", | ||
| content: { | ||
| "application/json": { | ||
| schema: { | ||
| type: "object", | ||
| properties: { error: { type: "string" } }, | ||
| required: ["error"], | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| "403": { | ||
| description: "Forbidden", | ||
| content: { | ||
| "application/json": { | ||
| schema: { | ||
| type: "object", | ||
| properties: { error: { type: "string" } }, | ||
| required: ["error"], | ||
| }, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Refactor duplicated error response schemas into reusable components
To improve maintainability and reduce duplication, consider defining a reusable ErrorResponse schema in the components section and referencing it in the error responses for status codes 500, 401, and 403.
Apply the following changes:
Add the ErrorResponse schema to components.schemas (after line 91):
+ schemas: {
+ ErrorResponse: {
+ type: "object",
+ properties: { error: { type: "string" } },
+ required: ["error"],
+ },
+ },Update the error responses to reference the ErrorResponse schema.
For the 500 response (lines 53-57):
For the 401 response (lines 65-69):
For the 403 response (lines 77-81):
This change centralizes the error response schema and enhances maintainability.
Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation